v2 remove console warns on middleware by KKonstantinov · Pull Request #1776 · modelcontextprotocol/typescript-sdk

KKonstantinov · 2026-03-26T22:26:04Z

Remove console.warn log pollution and add skipHostHeaderValidation option

Motivation and Context

Fixes #1515. The middleware packages (@modelcontextprotocol/express, @modelcontextprotocol/hono) emit a console.warn when binding to 0.0.0.0 or :: without allowedHosts. This pollutes server logs for users who intentionally bind to all interfaces (e.g., behind a reverse proxy in a container). Libraries should not produce unsolicited log output in environments they don't own.

Additionally, there was no way to opt out of the automatic localhost host-header validation — useful when running behind a reverse proxy that rewrites the Host header.

This PR:

Removes the console.warn entirely. The allowedHosts option is already well-documented in JSDoc.
Adds skipHostHeaderValidation — an explicit opt-out from all automatic host-header validation middleware.
Uses a discriminated union type (HostHeaderValidationOptions) so that allowedHosts and skipHostHeaderValidation: true are mutually exclusive at the type level — passing both is a compile error, not a silent precedence rule.

How Has This Been Tested?

All existing tests updated and passing for both @modelcontextprotocol/express and @modelcontextprotocol/hono
New tests added for skipHostHeaderValidation: true on both localhost and 0.0.0.0 hosts
Hono tests verify at the HTTP level that validation middleware is actually skipped (requests with arbitrary Host headers succeed)
Full build, typecheck, and lint pass across both packages

Breaking Changes

The console.warn when binding to 0.0.0.0/:: without allowedHosts is removed. This is a behavior change but not a breaking API change — no user code depended on this warning.
CreateMcpExpressAppOptions and CreateMcpHonoAppOptions changed from interface to type (intersection with HostHeaderValidationOptions). This is source-compatible for all usage patterns except extends/implements on the interface, which is unlikely for options objects.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

Additional context

The HostHeaderValidationOptions discriminated union type is defined independently in both packages (no shared import) to avoid coupling the middleware packages. The pattern is identical:

type HostHeaderValidationOptions =
    | { skipHostHeaderValidation: true; allowedHosts?: never }
    | { skipHostHeaderValidation?: false; allowedHosts?: string[] };

This ensures that allowedHosts is only accepted when validation is enabled, catching misconfiguration at compile time rather than silently ignoring one option.

…ning

changeset-bot · 2026-03-26T22:26:08Z

⚠️ No Changeset found

Latest commit: 28fd4ad

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

pkg-pr-new · 2026-03-26T22:27:31Z

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@1776

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@1776

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@1776

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@1776

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@1776

commit: 28fd4ad

packages/middleware/express/src/express.ts

packages/middleware/express/test/express.test.ts

…extprotocol/typescript-sdk into middleware-remove-console-warn

KKonstantinov · 2026-03-27T05:49:52Z

@claude review

claude

The previously flagged integration test bug has been fixed. The implementation looks correct, but this PR adds a new option to bypass DNS rebinding protection, so a human should sign off on the API design.

Extended reasoning...

Overview

This PR modifies the Express and Hono middleware packages to (1) remove console.warn log pollution when binding to 0.0.0.0/:: without allowedHosts, and (2) add a new skipHostHeaderValidation option using a discriminated union type that makes allowedHosts and skipHostHeaderValidation: true mutually exclusive at the type level. Five files are changed across both middleware packages and integration tests.

Security risks

The core change introduces an explicit opt-in to disable DNS rebinding protection entirely via skipHostHeaderValidation: true. While the implementation is correct (requires explicit opt-in, default behavior unchanged, clean type constraints), this is a new security-relevant API surface. A human reviewer should validate that this escape hatch is appropriate and that the documentation/naming adequately communicates the security implications.

Level of scrutiny

This warrants careful human review. Although the code is well-structured and the discriminated union type prevents misconfiguration at compile time, the PR touches security middleware (host header validation / DNS rebinding protection). The interface-to-type change is also technically a minor breaking change for anyone extending the options interface. A missing changeset was also flagged by the changeset bot.

Other factors

The author addressed the integration test bug I previously flagged — the old console.warn assertion tests are now replaced with proper HTTP-level supertest assertions. The nit about Express unit tests only checking toBeDefined() is mitigated by the new integration tests which do verify HTTP behavior with supertest. Test coverage is now solid across both packages.

claude · 2026-03-27T14:22:37Z

test/integration/test/server.test.ts

+    test('should not apply host validation for 0.0.0.0 without allowedHosts', async () => {
+        const app = createMcpExpressApp({ host: '0.0.0.0' });
+        app.post('/test', (_req: Request, res: Response) => {
+            res.json({ success: true });
+        });
+
+        // No host validation applied, so any host should be accepted
+        const response = await supertest(app).post('/test').set('Host', 'anything.com:3000').send({});
+        expect(response.status).toBe(200);
    });


🟡 Nit: The new test at line 2311 ("should not apply host validation for 0.0.0.0 without allowedHosts") is functionally identical to the pre-existing test at line 2262 ("should not apply host validation when host is 0.0.0.0") — both create an app with host: '0.0.0.0', POST with an arbitrary Host header, and assert 200. This should instead cover the :: (IPv6 all-interfaces) case, which lost its test coverage when the old console.warn test was removed.

Extended reasoning...

The Duplicate

The test at line 2262 ("should not apply host validation when host is 0.0.0.0") and the new test at line 2311 ("should not apply host validation for 0.0.0.0 without allowedHosts") are functionally identical:

Both call createMcpExpressApp({ host: '0.0.0.0' })

Both add a POST /test handler that returns { success: true }

Both send a request with an arbitrary Host header (any-host.com:3000 vs anything.com:3000)

Both assert response.status is 200

The only differences are the test name, the specific host header string used, and the older test also asserts response.body.

How This Happened

Before this PR, there were two console.warn tests — one for 0.0.0.0 (line 2311 in the old code) and one for :: (line 2318 in the old code). The PR replaced both with behavioral tests, but the replacement for the 0.0.0.0 warn test duplicates the pre-existing behavioral test at line 2262 instead of testing something new.

Step-by-Step Proof

Line 2262: createMcpExpressApp({ host: '0.0.0.0' }) → POST /test with Host: any-host.com:3000 → assert 200

Line 2311: createMcpExpressApp({ host: '0.0.0.0' }) → POST /test with Host: anything.com:3000 → assert 200

These exercise the exact same code path: host is '0.0.0.0', which is not in localhostHosts, so no validation middleware is applied, and any Host header is accepted.

Lost Coverage

The old test "should warn when binding to :: (IPv6 all interfaces)" was the only test exercising the :: host case. When it was removed, no replacement test for host: '::' was added. The :: case follows the same code path as 0.0.0.0 (not in localhostHosts, no validation applied), but it still deserves its own test to confirm IPv6 all-interfaces behavior.

Impact

This is a minor issue — both tests pass fine and no incorrect behavior results. It is test redundancy and a missed opportunity to cover the :: case.

Suggested Fix

Change the test at line 2311 to use host: '::' instead of host: '0.0.0.0', making it cover the IPv6 all-interfaces case that lost its test.

add skipostHeaderValidation option, default false, remove console war…

d75a414

…ning

Merge branch 'main' into middleware-remove-console-warn

e44e301

KKonstantinov marked this pull request as ready for review March 26, 2026 22:27

KKonstantinov requested a review from a team as a code owner March 26, 2026 22:27

claude bot reviewed Mar 26, 2026

View reviewed changes

packages/middleware/express/src/express.ts Show resolved Hide resolved

packages/middleware/express/test/express.test.ts Show resolved Hide resolved

KKonstantinov added 2 commits March 27, 2026 00:41

test fixes

d920205

Merge branch 'middleware-remove-console-warn' of github.com:modelcont…

9ee628f

…extprotocol/typescript-sdk into middleware-remove-console-warn

claude bot reviewed Mar 27, 2026

View reviewed changes

Merge branch 'main' into middleware-remove-console-warn

28fd4ad

claude bot reviewed Mar 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v2 remove console warns on middleware#1776

v2 remove console warns on middleware#1776
KKonstantinov wants to merge 5 commits intomainfrom
middleware-remove-console-warn

KKonstantinov commented Mar 26, 2026

Uh oh!

changeset-bot bot commented Mar 26, 2026 •

edited

Loading

Uh oh!

pkg-pr-new bot commented Mar 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

KKonstantinov commented Mar 27, 2026

Uh oh!

claude bot left a comment

Uh oh!

claude bot Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

KKonstantinov commented Mar 26, 2026

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Checklist

Additional context

Uh oh!

changeset-bot bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

pkg-pr-new bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KKonstantinov commented Mar 27, 2026

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

claude bot Mar 27, 2026

Choose a reason for hiding this comment

The Duplicate

How This Happened

Step-by-Step Proof

Lost Coverage

Impact

Suggested Fix

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

changeset-bot bot commented Mar 26, 2026 •

edited

Loading

pkg-pr-new bot commented Mar 26, 2026 •

edited

Loading